hardhat-scripts/deploy/2.roles.ts (1)

99-103: WATCHER_ROLE not granted to MessageSwitchboard (bug).

The grant logic omits MessageSwitchboard from the condition that routes WATCHER_ROLE to the watcher address. This causes WATCHER_ROLE to be incorrectly granted to signer.address for MessageSwitchboard.

Apply this diff:

-        [Contracts.FastSwitchboard, Contracts.CCTPSwitchboard].includes(
+        [Contracts.FastSwitchboard, Contracts.CCTPSwitchboard, Contracts.MessageSwitchboard].includes(
           contractName as Contracts
         ) && roleName === ROLES.WATCHER_ROLE

src/finality.ts (1)

7-7: Past LOW=0 concern not addressed

The previous review flagged that LOW: 0 creates reorg exposure and falsy-defaulting bugs. This change from 1→0 remains unaddressed. Either document this as intentional "head-of-chain" semantics with explicit consumer guidance to use ?? instead of ||, or revert to 1 for safer defaults.

hardhat-scripts/verify/verify.ts (1)

119-135: Retry loop still re-verifies all contracts and sleeps unconditionally.

The issues flagged in past review comments remain:

1. The loop doesn't narrow the pending set between retries—it keeps re-verifying chainParams instead of unverifiedChainParams.
2. The sleep happens before checking if verification is complete.

Since the loop currently runs only once (line 119), these issues don't cause problems yet. However, if you increase the retry limit in the future, apply the fixes from the past review comments to avoid unnecessary work and delays.

hardhat-scripts/constants/types.ts (1)

17-17: Type switchboardId as BigNumberish for uint64 safety.

As noted in previous review, using string for a uint64 value is too permissive and can cause runtime errors. BigNumberish provides proper type safety for ethers.js calls.

Apply this diff:

+import { BigNumberish } from "ethers";
+
 export type AppGatewayConfig = {
   plugConfig: {
     appGatewayId: string;
-    switchboardId: string;
+    switchboardId: BigNumberish;
   };
   plug: string;
   chainSlug: number;
 };

hardhat-scripts/utils/address.ts (2)

58-65: Simplify toBytes32Format.

The function redundantly strips 0x prefix when toBytes32FormatHexString should always return a 0x-prefixed string after fixing the previous issue.

Apply this diff:

 export function toBytes32Format(hexString: string): number[] {
-  const hex32Format = toBytes32FormatHexString(hexString);
-  const cleanedHex32String = hex32Format.startsWith("0x")
-    ? hex32Format.slice(2)
-    : hex32Format;
-
-  return Array.from(Buffer.from(cleanedHex32String, "hex"));
+  const hex32 = toBytes32FormatHexString(hexString);
+  return Array.from(Buffer.from(hex32.slice(2), "hex"));
 }

---

42-56: Fix padding/validation for toBytes32FormatHexString.

The function has multiple edge-case issues:

• Odd-length hex produces fractional offset, silently dropping nibbles
• Non-hex input isn't validated
• Inputs longer than 32 bytes aren't checked
• Returns input unchanged for 64-char strings without ensuring 0x prefix

This should strictly validate hex, left-pad to 32 bytes, and always return 0x-prefixed output.

Apply this diff:

-export function toBytes32FormatHexString(hexString: string): string {
-  // this means that the string is already in bytes32 format with or without 0x prefix
-  if (hexString.length == 64 || hexString.length == 66) {
-    return hexString;
-  }
-  // Remove the '0x' prefix from the input string if it's present
-  const cleanedHexString = hexString.startsWith("0x")
-    ? hexString.slice(2)
-    : hexString;
-
-  const buffer = Buffer.alloc(32);
-  buffer.write(cleanedHexString, 32 - cleanedHexString.length / 2, "hex"); // each hex char is 2 bytes
-
-  return "0x" + buffer.toString("hex");
-}
+export function toBytes32FormatHexString(input: string): string {
+  // Normalize and strip 0x
+  let hex = input.startsWith("0x") ? input.slice(2) : input;
+  // Validate hex
+  if (!/^[0-9a-fA-F]*$/.test(hex)) {
+    throw new Error(`Invalid hex string: ${input}`);
+  }
+  // Enforce max size
+  if (hex.length > 64) {
+    throw new Error(`Hex string too long for bytes32 (got ${hex.length} nybbles)`);
+  }
+  // Enforce even length
+  if (hex.length % 2 !== 0) {
+    hex = "0" + hex;
+  }
+  // Left-pad to 32 bytes (64 nybbles)
+  return "0x" + hex.padStart(64, "0").toLowerCase();
+}

contracts/protocol/switchboard/SwitchboardBase.sol (1)

38-40: Add duplicate registration guard and event emission.

registerSwitchboard can be called multiple times, overwriting switchboardId without validation or notification. This should prevent duplicate registrations and emit an event for off-chain tracking.

Apply this diff:

+    event SwitchboardRegistered(uint64 indexed switchboardId);
+
     function registerSwitchboard() external onlyOwner {
+        require(switchboardId == 0, "Already registered");
         switchboardId = socket__.registerSwitchboard();
+        emit SwitchboardRegistered(switchboardId);
     }

hardhat-scripts/constants/constants.ts (1)

14-15: Document units for MSG_SB_FEES (keep string BigNumberish)

This should state wei units to prevent misuse. Keep string for BigNumberish compatibility.

-export const MSG_SB_FEES = "100000000";
+// Switchboard sibling-config fee (wei); keep as string for BigNumberish compatibility.
+export const MSG_SB_FEES = "100000000";

contracts/protocol/interfaces/ISwitchboard.sol (1)

19-26: Fix param docs order and document payable rationale

Docs list triggerId_ before plug_, but signature takes plug_ first. This should match the function to avoid integration errors. Also add a clear note why this is payable (fees/gas).

-    /**
-     * @notice Processes a trigger and creates payload
-     * @dev This function is called by the socket to process a trigger
-     * @dev sb can override this function to add additional logic
-     * @param triggerId_ Trigger ID from socket
-     * @param plug_ Source plug address
-     * @param payload_ Payload data
-     * @param overrides_ Overrides for the trigger
-     */
+    /**
+     * @notice Processes a trigger and creates payload.
+     * @dev Called by Socket; switchboards can implement custom logic.
+     * @dev Payable to collect switchboard fees and/or gas prepayment; implementers must document exact requirements.
+     * @param plug_ Source plug address
+     * @param triggerId_ Trigger ID from Socket
+     * @param payload_ Payload data
+     * @param overrides_ Overrides for the trigger
+     */

package.json (1)

25-25: Guard .env and avoid relying on executable bit

The trace script will fail when .env is absent and is shell‑specific. This should guard the source and invoke via bash for portability.

-    "trace": "source .env && bash trace.sh",
+    "trace": "bash -lc '[[ -f .env ]] && source .env; bash trace.sh'",

hardhat-scripts/utils/overrides.ts (1)

7-7: defaultType still forces legacy transactions (type 0).

The hardcoded defaultType = 0 at Line 7 forces legacy transactions on all networks where an explicit type isn't set. This prevents EIP-1559 transactions on networks that prefer or require them. The getOverrides function at Line 112 still uses this default.

Remove the default type to let ethers infer the appropriate transaction type:

-const defaultType = 0;
 
-  const { gasLimit, type = defaultType } = overrides;
+  const { gasLimit, type } = overrides;

Also applies to: 112-112

script/counter/DeployEVMxCounterApp.s.sol (1)

19-19: Fees remain hardcoded at 1 ether.

This was previously flagged. Fees should be sourced from an environment variable (e.g., FEES_WEI) to avoid misconfiguration across networks/forks.

-        uint256 fees = 1 ether;
+        uint256 fees = vm.envUint("FEES_WEI");

contracts/protocol/SocketConfig.sol (1)

123-132: Address the interface mismatch in connect() function.

The past review comment is still valid. The function signature doesn't match the ISocket interface. The interface expects connect(bytes32 appGatewayId_, address socket_, uint64 switchboardId_) but this implementation omits the socket_ parameter.

Apply this diff to fix the signature:

-    function connect(bytes32 appGatewayId_, uint64 switchboardId_) external override {
+    function connect(bytes32 appGatewayId_, address socket_, uint64 switchboardId_) external override {
         if (isValidSwitchboard[switchboardId_] != SwitchboardStatus.REGISTERED)
             revert InvalidSwitchboard();

-        PlugConfigEvm storage _plugConfig = _plugConfigs[msg.sender];
+        PlugConfigEvm storage _plugConfig = _plugConfigs[socket_];
         _plugConfig.appGatewayId = appGatewayId_;
         _plugConfig.switchboardId = switchboardId_;

-        emit PlugConnected(msg.sender, appGatewayId_, switchboardId_);
+        emit PlugConnected(socket_, appGatewayId_, switchboardId_);
     }

contracts/evmx/watcher/Watcher.sol (1)

183-201: tryCall error handling loses revert reason.

The past review comment correctly identified that the current implementation only checks the success boolean and discards the returnData from tryCall (lines 193-199). This makes debugging failures difficult since the actual revert reason from the called contract is lost.

test/evmx/AuctionManager.t.sol (1)

33-51: Test lacks outcome validation.

This test executes operations but doesn't verify results. The past review comments correctly identified:

1. Missing balance/state checks after withdrawCredits (lines 39-46)
2. Missing assertions after executeRequest calls to verify bid expiry and state changes (lines 47-51)

Without assertions, the test can't detect failures in the withdrawal or bid expiry logic.

script/helpers/TransferRemainingCredits.s.sol (1)

28-37: Missing validation before transfer.

The past review comment correctly identified that the script doesn't check if availableFees > 0 before attempting the transfer (lines 28-37). This results in unnecessary transaction costs for no-op transfers.

contracts/protocol/switchboard/FastSwitchboard.sol (2)

43-43: Changing visibility from external to public increases gas; keep external (or external virtual)

attest is only used externally; public adds unnecessary ABI thunk. This should be external (and virtual if overriding).

-    function attest(bytes32 digest_, bytes calldata proof_) public virtual {
+    function attest(bytes32 digest_, bytes calldata proof_) external virtual {

---

64-72: Empty processTrigger silently accepts ETH and does nothing

An external payable function with an empty body will consume gas/ETH and do nothing. This should be implemented, made abstract, or revert explicitly to prevent fund loss and logic gaps.

Options:

-    function processTrigger(
+    function processTrigger(
         address plug_,
         bytes32 triggerId_,
         bytes calldata payload_,
         bytes calldata overrides_
-    ) external payable virtual {}
+    ) external payable virtual {
+        revert("NotImplemented");
+    }

Or mark the contract abstract and declare the function without a body if derived contracts must implement it.

contracts/evmx/fees/FeesManager.sol (1)

51-67: Reinitializer bump requires an explicit upgrade/migration path

reinitializer(2) indicates an upgrade. Existing deployments will skip v1 initializer; new state must be safely initialized and storage layout preserved. This should include migration logic, upgrade tests, and documentation.

Add/point to:

• Upgrade script that calls the new initializer post‑proxy upgrade.
• A test that deploys v1, upgrades to v2, validates maxFeesPerChainSlug, sbType, feesPool, and owner/app gateway state.

contracts/protocol/SocketBatcher.sol (3)

13-19: Move IFastSwitchboard to interfaces file

Interfaces should live under interfaces/ for reuse and discoverability.

Create contracts/protocol/interfaces/IFastSwitchboard.sol and import it here.

---

47-66: Validate switchboardId resolves to a non-zero address before attest

This should guard against invalid IDs and clearer errors.

-        IFastSwitchboard(socket__.switchboardAddresses(switchboardId_)).attest(digest_, proof_);
+        address switchboard = socket__.switchboardAddresses(switchboardId_);
+        if (switchboard == address(0)) revert InvalidSwitchboard();
+        IFastSwitchboard(switchboard).attest(digest_, proof_);

---

76-104: CCTP: add zero-address check, try/catch around attest/prove, and syncOut only on success

This should improve safety and diagnostics; syncing on failure risks inconsistency.

-    address switchboard = socket__.switchboardAddresses(switchboardId_);
+    address switchboard = socket__.switchboardAddresses(switchboardId_);
+    if (switchboard == address(0)) revert InvalidSwitchboard();
...
-    ICCTPSwitchboard(switchboard).attestVerifyAndProveExecutions(
-        execParams_,
-        cctpParams_,
-        payloadId
-    );
+    try ICCTPSwitchboard(switchboard).attestVerifyAndProveExecutions(
+        execParams_,
+        cctpParams_,
+        payloadId
+    ) {
+        // ok
+    } catch Error(string memory reason) {
+        revert(string.concat("CCTP attest/prove failed: ", reason));
+    } catch {
+        revert("CCTP attest/prove failed");
+    }
...
-    ICCTPSwitchboard(switchboard).syncOut(payloadId, cctpParams_.nextBatchRemoteChainSlugs);
-    return (success, returnData);
+    if (success) {
+        ICCTPSwitchboard(switchboard).syncOut(payloadId, cctpParams_.nextBatchRemoteChainSlugs);
+    }
+    return (success, returnData);

contracts/evmx/base/AppGatewayBase.sol (3)

125-130: Validate request count in onDeployComplete to prevent TOCTOU

You encode watcher__().getCurrentRequestCount() but ignore it in the callback. This should validate the request count to avoid races between deploy and completion.

Apply:

-    function onDeployComplete(uint40, bytes calldata onCompleteData_) external virtual onlyWatcher {
+    function onDeployComplete(uint40 requestCount_, bytes calldata onCompleteData_) external virtual onlyWatcher {
+        require(requestCount_ <= watcher__().getCurrentRequestCount(), "Invalid request count");
         if (onCompleteData_.length == 0) return;
         uint32 chainSlug = abi.decode(onCompleteData_, (uint32));
         initializeOnChain(chainSlug);
     }

Also applies to: 373-377

---

145-150: Validate decoded deployed address before converting to bytes32

This should reject address(0) before toBytes32Format to avoid storing invalid forwarders.

 function setAddress(bytes memory data_, bytes memory returnData_) external onlyPromises {
     (uint32 chainSlug, bytes32 contractId) = abi.decode(data_, (uint32, bytes32));
-    forwarderAddresses[contractId][chainSlug] = asyncDeployer__().getOrDeployForwarderContract(
-        toBytes32Format(abi.decode(returnData_, (address))),
-        chainSlug
-    );
+    address deployed = abi.decode(returnData_, (address));
+    require(deployed != address(0), "Invalid deployed address");
+    forwarderAddresses[contractId][chainSlug] = asyncDeployer__().getOrDeployForwarderContract(
+        toBytes32Format(deployed),
+        chainSlug
+    );
 }

---

226-228: Guard against zero on-chain plug address

Registering a zero plug degrades watcher state and later calls.

 function _setValidPlug(bool isValid, uint32 chainSlug_, bytes32 contractId_) internal {
     bytes32 onchainAddress = getOnChainAddress(contractId_, chainSlug_);
+    require(onchainAddress != bytes32(0), "Invalid onchain address");
     watcher__().setIsValidPlug(isValid, chainSlug_, onchainAddress);
 }

hardhat-scripts/deploy/3.configureChains.ts (2)

50-57: Do not overwrite existing SwitchboardIdToAddressMap

This resets mappings and loses prior state. Initialize only when undefined.

-    let deployUtils: DeployParams = {
+    let deployUtils: DeployParams = {
       addresses: chainAddresses,
       mode,
       signer: signer,
       currentChainSlug: chain as ChainSlug,
     };
-    deployUtils.addresses.SwitchboardIdToAddressMap = {};
+    deployUtils.addresses.SwitchboardIdToAddressMap =
+      deployUtils.addresses.SwitchboardIdToAddressMap || {};

---

61-72: Validate returned fastSwitchboardId before storing

This should fail fast if registration/query returns zero/invalid.

   const fastSwitchboardId = await registerSb(
     chain,
     chainAddresses[Contracts.FastSwitchboard],
     signer,
     socketContract
   );
+  if (!fastSwitchboardId || fastSwitchboardId.toString() === "0") {
+    throw new Error(`Failed to register FastSwitchboard for chain ${chain}`);
+  }
   deployUtils.addresses[Contracts.FastSwitchboardId] =
     fastSwitchboardId.toString();

hardhat-scripts/deploy/6.connect.ts (1)

48-56: Validate switchboardId before using it

This should reject missing/zero IDs and prevent bad socket config calls.

-  const switchboardId = addr[Contracts.FastSwitchboardId];
+  const switchboardId = addr[Contracts.FastSwitchboardId];
+  if (!switchboardId || switchboardId === "0") {
+    throw new Error(`FastSwitchboardId not found for chain ${chain}`);
+  }
...
   const tx = await plug.functions[functionName](
     appGatewayId,
     socket.address,
     switchboardId,

Also applies to: 68-73

contracts/evmx/AuctionManager.sol (1)

235-246: then() inside async-wrapped function attaches to the previous promise.

With internal async, _postAsync() creates the new Promise after the function body. Calling then(...) here registers on the old promise.

Fix one of:

• Move then(callbackSelector_, callbackData_) to be called immediately after invoking _createRequest(...) at the call sites.
• Or extend the async modifier/post-hook to accept (selector, data) and call IPromise(watcher__().latestAsyncPromise()).then(selector, data) inside _postAsync().

Minimal change:

-        _setSchedule(delayInSeconds_);
-        then(callbackSelector_, callbackData_);
+        _setSchedule(delayInSeconds_);
+        // Attach .then(...) to the newly created promise after this function returns.

hardhat-scripts/deploy/1.deploy.ts (1)

122-124: Fix optional chaining check for feePool (regression).

feePool?.length == 0 skips deployment when feePool is undefined. This should deploy when missing or empty.

-      const feePool = getFeePool();
-      if (feePool?.length == 0) {
+      const feePool = getFeePool();
+      if (!feePool || feePool.length === 0) {

Errors.md (1)

52-56: Doc inconsistency: removed error name mismatch.

Summary claims removal of SwitchboardExistsOrDisabled(), table lists SwitchboardExists(). This should be consistent.

Update either the summary or the table so both reference the same removed error name/signature.

contracts/evmx/watcher/precompiles/WritePrecompile.sol (1)

183-186: Validate and mask payloadPointer-derived fields before use.

Direct shifts to uint40(...) without masking/validation can yield garbage and corrupt prevBatchDigestHash.

-        bytes32 prevBatchDigestHash = getPrevBatchDigestHash(
-            uint40(payloadParams.payloadPointer >> 120),
-            uint40(payloadParams.payloadPointer >> 80)
-        );
+        uint40 requestCount = uint40((payloadParams.payloadPointer >> 120) & bytes32(uint256((1 << 40) - 1)));
+        uint40 batchCount = uint40((payloadParams.payloadPointer >> 80) & bytes32(uint256((1 << 40) - 1)));
+        if (requestCount == 0) revert InvalidIndex(); // or a dedicated error
+        // Optionally validate batchCount against known bounds
+        bytes32 prevBatchDigestHash = getPrevBatchDigestHash(requestCount, batchCount);

Pick the appropriate error type for out-of-range values (e.g., a dedicated InvalidRequestCount()).

test/evmx/FeesTest.t.sol (1)

96-103: Add explicit post-disconnect assertions

This should assert the plug is disconnected before proceeding (e.g., getPlugConfig returns appGatewayId == 0). Without it, the test could pass due to unrelated conditions.

Apply this snippet after disconnectSocket():

 hoax(socketOwner);
 arbConfig.feesPlug.disconnectSocket();
+
+// Verify disconnection state
+{
+    (bytes32 appGatewayId_, uint64 switchboardId_) =
+        arbConfig.socket.getPlugConfig(address(arbConfig.feesPlug));
+    assertEq(appGatewayId_, bytes32(0), "appGatewayId should be zero after disconnect");
+    // If protocol semantics keep switchboardId, assert unchanged; else enforce zero:
+    // assertEq(switchboardId_, 0, "switchboardId should be zero after disconnect");
+}

contracts/utils/common/Structs.sol (1)

213-219: Bound dynamic arrays to prevent DoS in consumers

CCTPBatchParams and SolanaInstruction* structs expose unbounded arrays. This should have enforced limits in consuming contracts (batch sizes, accounts, args) with clear reverts to prevent gas griefing.

• Define MAX_BATCH_SIZE, MAX_ACCOUNTS, MAX_ARGUMENTS constants and validate in entry points that accept these structs.
• Validate related lengths (e.g., attestations/messages alignment, accountFlags length == accounts length, functionArgumentTypeNames length == functionArguments length).

Also applies to: 220-239

test/SetupTest.t.sol (2)

1158-1173: setupGatewayAndPlugs: remove double-iteration; build configs in one pass

This currently scans contractIds_ twice. Build configs once into a temp array and resize.

-        uint256 validPlugCount = _countValidPlugs(appGateway_, contractIds_, chainSlug_);
-        AppGatewayConfig[] memory configs = new AppGatewayConfig[](validPlugCount);
-        _populateConfigs(configs, appGateway_, contractIds_, chainSlug_);
-        if (validPlugCount > 0) {
-            watcherMultiCall(
-                address(configurations),
-                abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs)
-            );
-        }
+        AppGatewayConfig[] memory tmp = new AppGatewayConfig[](contractIds_.length);
+        uint256 count = 0;
+        uint64 switchboardId = configurations.switchboards(chainSlug_, appGateway_.sbType());
+        for (uint i = 0; i < contractIds_.length; i++) {
+            bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_);
+            if (plug != bytes32(0)) {
+                tmp[count++] = AppGatewayConfig({
+                    plug: plug,
+                    chainSlug: chainSlug_,
+                    plugConfig: PlugConfigGeneric({
+                        appGatewayId: toBytes32Format(address(appGateway_)),
+                        switchboardId: switchboardId
+                    })
+                });
+            }
+        }
+        if (count > 0) {
+            AppGatewayConfig[] memory configs = new AppGatewayConfig[](count);
+            for (uint i = 0; i < count; i++) configs[i] = tmp[i];
+            watcherMultiCall(
+                address(configurations),
+                abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs)
+            );
+        }

Also applies to: 1175-1186, 1188-1211

---

1105-1108: Fix abi.decode type: last tuple element is uint64 (switchboardId), not address

This currently causes a type mismatch and/or incorrect decoding.

-            (, Transaction memory transaction, , , , ) = abi.decode(
-                params.precompileData,
-                (address, Transaction, WriteFinality, uint256, uint256, address)
-            );
+            (, Transaction memory transaction, , , , ) = abi.decode(
+                params.precompileData,
+                (address, Transaction, WriteFinality, uint256, uint256, uint64)
+            );

contracts/evmx/watcher/RequestHandler.sol (2)

255-258: payloadPointer packing can overflow allocated bit ranges

requestCount_ and nextBatchCount must fit 40 bits; payloadCounter must fit 80 bits. Without checks/masking, fields can overflow and corrupt the pointer.

+        // Ensure values fit within 40/40/80 bits respectively
+        require(requestCount_ < (1 << 40), "Request count overflow");
+        require(nextBatchCount < (1 << 40), "Batch count overflow");
+        require(payloadCounter   < (1 << 80), "Payload counter overflow");
-        uint160 payloadPointer = (uint160(requestCount_) << 120) |
-            (uint160(nextBatchCount) << 80) |
-            uint160(payloadCounter++);
+        uint160 payloadPointer =
+            (uint160(requestCount_)   << 120) |
+            (uint160(nextBatchCount)  << 80)  |
+            uint160(payloadCounter++);

---

428-431: Capture and emit failure data from tryCall in onComplete

Only emitting RequestCompletedWithErrors loses error context. This should capture returnData and emit it.

-        if (r.onCompleteData.length > 0) {
-            (bool success, , ) = r.appGateway.tryCall(0, gasleft(), 0, r.onCompleteData);
-            if (!success) emit RequestCompletedWithErrors(requestCount_);
-        }
+        if (r.onCompleteData.length > 0) {
+            (bool success, bytes memory returnData, ) =
+                r.appGateway.tryCall(0, gasleft(), 0, r.onCompleteData);
+            if (!success) {
+                // Consider updating the event to include bytes returnData
+                emit RequestCompletedWithErrors(requestCount_);
+                // Optionally emit returnData via a new event to aid debugging
+            }
+        }

EventTopics.md (1)

41-45: Empty sections should be removed or populated.

These empty interface sections (IFastSwitchboard, IMessageSwitchboard, ICCTPSwitchboard) have been flagged in previous reviews. If these interfaces have no events to document, remove the sections; otherwise, populate them with the actual event information.